Skip to content

Update gcc CI to use gcc-11 container #59202

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
BruceForstall merged 1 commit into from
Sep 16, 2021
Merged

Update gcc CI to use gcc-11 container #59202

BruceForstall merged 1 commit into from
Sep 16, 2021

Conversation

@am11
Copy link
Member

@am11 am11 commented Sep 16, 2021 •
edited
Loading

With gcc9 leg, we had 2500+ warnings in JIT due to a bit field width issue: #33541.

Now that we have a dedicated gcc container, it is easier to upgrade gcc toolchain independent from llvm toolchain.

This PR uses the latest gcc 11 prereq image. The bitfield width warnings have been fixed in this version, therefore I have enabled Werror in the JIT for gcc version >= 11.

Fixes #33541

@ghost
Copy link

ghost commented Sep 16, 2021

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Sep 16, 2021
@am11 am11 force-pushed the feature/gcc-11-in-ci branch 2 times, most recently from e135661 to 3de8738 Compare September 16, 2021 14:24
@am11 am11 marked this pull request as ready for review September 16, 2021 14:30
@am11
Copy link
Member Author

am11 commented Sep 16, 2021

cc @janvorli, @BruceForstall

am11
am11 commented Sep 16, 2021
View reviewed changes
eng/native/configurecompiler.cmake Outdated
Copy link
Member Author

@am11 am11 Sep 16, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This condition was missed out previously, and was failing the libs.native subset build with gcc (which we currently don't exercise in the CI and maybe we can; since it takes 30-60 seconds max)

@am11 am11 force-pushed the feature/gcc-11-in-ci branch from 3de8738 to 21756e6 Compare September 16, 2021 14:40
@am11
Copy link
Member Author

am11 commented Sep 16, 2021 •
edited
Loading

gcc leg has picked up the newer version:

Invoking "/__w/1/s/eng/native/gen-buildsys.sh" "/__w/1/s/src/coreclr" "/__w/1/s/artifacts/obj/coreclr/Linux.x64.Checked" x64 gcc "" "" Checked ""  -DCLR_CMAKE_PGO_INSTRUMENT=0 -DCLR_CMAKE_OPTDATA_PATH=/__w/1/s/.packages/optimization.linux-x64.pgo.coreclr/1.0.0-prerelease.21460.6 -DCLR_CMAKE_PGO_OPTIMIZE=1 -DFEATURE_DISTRO_AGNOSTIC_SSL=1 
/__w/1/s/artifacts/obj/coreclr/Linux.x64.Checked /__w/1/s
Not searching for unused variables given on the command line.
-- The C compiler identification is GNU 11.2.0
-- The CXX compiler identification is GNU 11.2.0
...

no compiler warnings. There is one preexisting assembler warning which remained, however:

[ 47%] Building CXX object jit/CMakeFiles/clrjit_unix_x64_x64.dir/bitset.cpp.o
/tmp/cchOQSqP.s: Assembler messages:
/tmp/cchOQSqP.s:3245: Warning: ignoring changed section attributes for .rodata
[ 47%] Building CXX object ildasm/exe/CMakeFiles/ildasm.dir/__/dis.cpp.o

I haven't looked too deep but I think it is not related to GSCookie, or it is benign. The compiled binaries do have all the exported symbols in correct segments which we typically find when compiled with clang (like the ones which SOS looks for).

janvorli
janvorli approved these changes Sep 16, 2021
View reviewed changes
Copy link
Member

@janvorli janvorli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you!

@BruceForstall BruceForstall merged commit b9926a5 into dotnet:main Sep 16, 2021
@am11 am11 deleted the feature/gcc-11-in-ci branch September 17, 2021 05:59
@ghost ghost locked as resolved and limited conversation to collaborators Nov 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@janvorli janvorli janvorli approved these changes

+1 more reviewer

@BruceForstall BruceForstall BruceForstall approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

GCC warns when compiling the JIT: too small to hold all values of regNumber

3 participants

@am11 @janvorli @BruceForstall